Skip to content

Add braces to prefetch_noexcept location var to fix clang-tidy reported error - #23066

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
davidwendt:prefetch-linter-fix
Jul 1, 2026
Merged

Add braces to prefetch_noexcept location var to fix clang-tidy reported error#23066
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
davidwendt:prefetch-linter-fix

Conversation

@davidwendt

@davidwendt davidwendt commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes a clang-tidy reported error:

cpp/src/utilities/prefetch.cpp:54:5: error: suggest braces around initialization of subobject [clang-diagnostic-missing-braces,-warnings-as-errors]
   54 |     device_id.value()};
      |     ^~~~~~~~~~~~~~~~~
      |     {                }

Reference: https://github.com/rapidsai/cudf/actions/runs/28523775732/job/84555399492?pr=23031#step:13:1460

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt self-assigned this Jul 1, 2026
@davidwendt
davidwendt requested a review from a team as a code owner July 1, 2026 15:08
@davidwendt
davidwendt requested review from mhaseeb123 and ttnghia July 1, 2026 15:08
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1ed75390-2644-49e1-937c-3f2c08ba8754

📥 Commits

Reviewing files that changed from the base of the PR and between a5dccda and 09f8b87.

📒 Files selected for processing (1)
  • cpp/src/utilities/prefetch.cpp
📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated internal CUDA handling to improve compatibility with newer CUDA versions.
    • Refreshed copyright year information.

Walkthrough

This change modifies the cudaMemLocation initializer in the CUDA 13+ code path of prefetch_noexcept within cpp/src/utilities/prefetch.cpp, wrapping device_id.value() in braces, and updates the SPDX copyright year to include 2026.

Changes

prefetch.cpp fix

Layer / File(s) Summary
cudaMemLocation initializer fix
cpp/src/utilities/prefetch.cpp		 The device component of the cudaMemLocation initializer in the CUDA 13+ path changes from device_id.value() to {device_id.value()}, and the SPDX copyright header is updated to 2020–2026.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Related Issues: None specified

Related PRs: None specified

Suggested labels: bug, cuda-13

Suggested reviewers: None specified

Poem:
A brace, a hop, a tiny leap,
Around the device ID we now keep,
CUDA 13 nods with care,
The memory location lands just there,
2026 stamped upon the page,
A rabbit's small fix, a tidy stage. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main code change and the clang-tidy issue it fixes.
Description check ✅ Passed The description matches the changeset by explaining the missing-braces clang-tidy fix in prefetch.cpp.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

bdice
bdice approved these changes Jul 1, 2026
View reviewed changes

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's fine but I don't see why it's needed.

@bdice

bdice commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

I got curious about why this had changed. We updated the latest ci-conda tag to CUDA 13.3, which triggered this.

In CUDA 13.2, cudaMemLocation had:

enum cudaMemLocationType type;
int id;

In CUDA 13.3, it became:

enum cudaMemLocationType type;
union {
    int id;
};

bdice
bdice reviewed Jul 1, 2026
View reviewed changes
Comment thread cpp/src/utilities/prefetch.cpp
Comment on lines 52 to +54
cudaMemLocation location{
(device_id.value() == cudaCpuDeviceId) ? cudaMemLocationTypeHost : cudaMemLocationTypeDevice,
device_id.value()};
{device_id.value()}};

@bdice bdice Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative fix that should work with CUDA 13.0-13.2 or CUDA 13.3+ is this (untested):

Suggested change
cudaMemLocation location{
(device_id.value() == cudaCpuDeviceId) ? cudaMemLocationTypeHost : cudaMemLocationTypeDevice,
device_id.value()};
{device_id.value()}};
cudaMemLocation location{};
location.type = (device_id.value() == cudaCpuDeviceId)
? cudaMemLocationTypeHost
: cudaMemLocationTypeDevice;
location.id = device_id.value();

@davidwendt davidwendt Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion appears to be a "declare and delay initialization" anti-pattern which prevents location from being a constant variable.

@davidwendt

davidwendt commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Claude gave me this explanation:

The cudaMemLocation aggregate (introduced in CUDA 13's driver_types.h) is initialized with a multi-line brace-init-list. Clang's brace-elision checker flags the second member (id) because the first initializer is a line-broken conditional expression, and wants each subobject's initializer explicitly braced rather than relying on elision.

@davidwendt

davidwendt commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@Matt711 Matt711 mentioned this pull request Jul 1, 2026
Merged
3 tasks
@rapids-bot
rapids-bot Bot merged commit 7149886 into NVIDIA:main Jul 1, 2026
140 checks passed
@davidwendt
davidwendt deleted the prefetch-linter-fix branch July 1, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bdice bdice bdice approved these changes
@vyasr vyasr vyasr approved these changes
@PointKernel PointKernel PointKernel approved these changes
@Matt711 Matt711 Matt711 approved these changes
@ttnghia ttnghia Awaiting requested review from ttnghia ttnghia was automatically assigned from rapidsai/cudf-cpp-codeowners
@mhaseeb123 mhaseeb123 Awaiting requested review from mhaseeb123 mhaseeb123 was automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@davidwendt davidwendt

Labels

3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@davidwendt @bdice @vyasr @PointKernel @Matt711